Skip to content

Action for building docker images automatically#1441

Merged
j143 merged 7 commits into
apache:mainfrom
j143:docker-action-pub
Jan 8, 2022
Merged

Action for building docker images automatically#1441
j143 merged 7 commits into
apache:mainfrom
j143:docker-action-pub

Conversation

@j143

@j143 j143 commented Nov 6, 2021

Copy link
Copy Markdown
Member

Tasks

  • 1. Run nightly build as cron job
  • 2. run test build for each commit
  • 3. version build for release version

How to Review?

Published artifact at https://hub.docker.com/r/return01/test-ghactions-systemds

Usage test:

janardhan@cloudshell:~ (ai-experiments-001)$ docker run -it --rm return01/test-ghactions-systemds
Hello from SystemDS
SystemDS Statistics:
Total execution time:           0.020 sec.

Secrets: DOCKERHUB_USER and DOCKERHUB_TOKEN
These two variables are already set by INFRA. No need to set from our end.

https://issues.apache.org/jira/browse/INFRA-22430

@Baunsgaard

Copy link
Copy Markdown
Contributor

I think we can split the images a bit and reduce builds.

  1. Build max once a day.
  2. Only build the test image if we change R dependencies or the pom.xml
  3. Only build the other images if there is changes in src.

Thanks for the help!

@j143 j143 requested a review from Baunsgaard November 24, 2021 16:19
@j143 j143 changed the title [WIP] Action for building docker images automatically Action for building docker images automatically Nov 24, 2021
Comment thread .github/workflows/docker-pub.yml Outdated
- name: Checkout
uses: actions/checkout@v2

- name: Set up QEMU

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QEMU is for ?

@j143 j143 Nov 26, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an add-on, incase we want to build multi arch images. Optional though!

https://github.com/docker/buildx/#building-multi-platform-images

usage

$ docker buildx build --platform linux/amd64,linux/arm64 .

I do not have a about whether we need this. Perhaps, federated feature
might need this (I do not have good understanding yet).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, then for now lets not include it if it is not needed

Comment thread docker/sysds.Dockerfile Outdated
rm -r target/maven-archiver && \
rm -r target/systemds-** && \
rm -r docker && \
# rm -r docker && \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot just comment out a line in this multi-line instruction, then everything after it does not execute.

@j143 j143 Nov 26, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# rm -r docker && \
rm -r docker && \

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean?
just for information the reason why i have this line is to remove the docker folder from the internal copied systemds repository, it does not make a difference for the building of the docker image except it makes the end result slightly smaller.

so therefore, don't out comment it :)

Comment thread docker/sysds.Dockerfile Outdated
COPY docker/mountFolder/main.dml /input/main.dml
RUN mkdir /input && echo 'print("Hello from SystemDS")' > /input/main.dml

# COPY docker/mountFolder/main.dml /input/main.dml

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm all for running it but don't remove the copy, so that the only change you make it to add the RUN line.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am having error with running COPY

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one

image

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is happening because of your out commented line mentioned above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one:

# rm -r docker && \

@Baunsgaard

Copy link
Copy Markdown
Contributor

I think it is fine if we just add the on commit part of the docker for now,
If it is still working, we should start merging it in.

@Baunsgaard

Copy link
Copy Markdown
Contributor

@j143
Is the only thing missing not just a Infra ticket to add keys to push the images?

@j143

j143 commented Jan 8, 2022

Copy link
Copy Markdown
Member Author

Is the only thing missing not just a Infra ticket to add keys to push the images?

There are already secrets available.

the problem seems to be COPY docker/mountFolder/main.dml /input/main.dml statement in the Dockerfile. Let me give a couple of hours to it. :)

#9 [4/4] COPY docker/mountFolder/main.dml /input/main.dml
#9 ERROR: failed to walk /tmp/buildkit-mount645512887/docker/mountFolder: lstat /tmp/buildkit-mount645512887/docker/mountFolder: no such file or directory

@j143 j143 merged commit de8a342 into apache:main Jan 8, 2022
@j143 j143 deleted the docker-action-pub branch January 8, 2022 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants